Emit error when backend unexpectedly disconnects #1316
Conversation
|
note: the native bindings behaved correctly and emitted errors on unexpected disconnections - only the pure JS driver needed changing. |
|
To simulate network partitions and backend disconnections I ended up implementing a fake PostgreSQL Server backend I could run within the integration test. Once this was implemented I could connect node-postgres clients to it and force terminations of the fake server. After I did this I was able to immediately reproduce the issue of clients 'dying' without emitting an error. |
|
This patch fixes the problem described for my app. Thanks! |
|
Great efforts @brianc !! Comparing to this, what I've done is implementing a setTimeout timer binding on connection instance, let it emit an error and then let error be fired through the path now the library already has. what do you think of this solution? is it helpful or appropriate to the issue? Appreciate for the comments if you would like to give :> |
Before this patch when the socket to the PostgreSQL server was terminated unexpectedly the client would emit a
closeevent but not emit anerrorevent. This means all clients in the pool would silently close without a user having any way to respond. This several problems around database failovers, silently "hanging" applications, etc.fixes #1113 #1075
also possibly fixes #967
I don't think we need to wait for 7.0 for this patch because it is fixing an incorrect behavior which causes silent application failures: it does not change any external APIs...so I think this is a backwards compatible change except sometimes applications which previously silently disconnected without reconnect or became unresponsive will now properly emit errors.